Add session/resume support for resuming previous sessions#248
Add session/resume support for resuming previous sessions#248farra wants to merge 2 commits intoxenodium:mainfrom
Conversation
This implements session/resume support as described in issue xenodium#105: - Check for `session.resume` capability during initialization - Add session persistence layer to save/load session metadata - Auto-save session IDs when creating new sessions - Add `agent-shell-resume-session` command to resume saved sessions - Add `agent-shell-delete-saved-session` command to manage saved sessions Session metadata is stored in `.agent-shell/sessions/` in JSON format including session ID, agent identifier, working directory, and timestamps. When resuming fails (e.g., session expired), gracefully falls back to creating a new session and cleans up the stale session file. New customization options: - `agent-shell-session-persistence-enabled`: Toggle session persistence - `agent-shell-sessions-directory-function`: Customize sessions directory Requires acp.el with `acp-make-session-resume-request` support. Closes xenodium#105 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The ACP protocol indicates session capabilities (resume, fork, list) by the
presence of a key with an empty object {}, not by a boolean true value.
For example: { "sessionCapabilities": { "resume": {} } }
When Emacs parses this JSON with json-read, the empty object {} becomes nil:
{"resume": {}} -> ((resume)) ; key 'resume' with value nil
The previous code used:
(and (map-elt session-capabilities 'resume) t)
This always returned nil because (map-elt ... 'resume) returned nil (the
parsed empty object), and (and nil t) evaluates to nil. This caused session
resume to always fall back to creating a new session.
The fix uses map-contains-key to check for key existence rather than value
truthiness:
(map-contains-key session-capabilities 'resume)
This correctly returns t when the key exists, even if its value is nil.
Also refactors agent-shell--resume-session to:
- First call agent-shell--initiate-handshake to discover capabilities
- Then check if resume is supported before attempting session/resume
- Extract the actual resume logic into agent-shell--do-session-resume
Fixes session resumption not working despite agent supporting it.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
xenodium
left a comment
xenodium
left a comment
There was a problem hiding this comment.
Thank you for the PR! I've had a first pass and left some minor comments in changes.
You likely noticed agent-shell.el is getting pretty chunky. I've started breaking agent-shell.el logic into related subpackages. For example c602039 Mind adding this feature in agent-shell-session.el?
Resuming sessions being an experimental ACP feature, it will possibly surface bugs (in either agent-shell or the agents themselves). It may result in folks filing related Github issues. Would you be keen to share the load in maintenance for this piece of code? No need to monitor the incoming issues, but I can cc you whenever needed. Does that work?
| (when (and agent-shell-session-persistence-enabled session-id) | ||
| (let* ((dir (funcall agent-shell-sessions-directory-function)) | ||
| (filepath (agent-shell--session-file-path session-id)) | ||
| (metadata `((sessionId . ,session-id) |
There was a problem hiding this comment.
Can we use self-evaluating symbols? That is start with :. Also - please, like we do for state. https://github.com/xenodium/agent-shell?tab=readme-ov-file#maps-use-alists
| (progn | ||
| (make-directory dir t) | ||
| (with-temp-file filepath | ||
| (insert (json-serialize metadata))) |
There was a problem hiding this comment.
We should consider saving as a printed elisp data structure:
(with-temp-file filepath
(prin1 metadata (current-buffer)))| "Update the lastAccessedAt timestamp for SESSION-ID." | ||
| (when-let ((metadata (agent-shell--load-session-metadata session-id))) | ||
| (let ((filepath (agent-shell--session-file-path session-id))) | ||
| (setf (alist-get 'lastAccessedAt metadata) |
There was a problem hiding this comment.
Can we use map.el here? https://github.com/xenodium/agent-shell?tab=readme-ov-file#mapel
| (defun agent-shell--format-session-for-display (session) | ||
| "Format SESSION metadata for display in completion." | ||
| (let* ((id (alist-get 'sessionId session)) | ||
| (agent (alist-get 'agentIdentifier session)) |
There was a problem hiding this comment.
Please prefer map.el here and elsewhere (ie. alist-get instances) is possible https://github.com/xenodium/agent-shell?tab=readme-ov-file#mapel
|
ps. Looks like there are some conflicts to resolve ;) |
|
Awesome! I'll do a review on my end today. And yes, happy to help maintain this. |
|
Thank you |
|
In case you haven't seen #263, I'm comparing the two PRs and seeing if merging feature-set is possible. May take me a little while to figure both out. For now, no need to update the PR while I look into it. |
|
Heads-up, we may be able to do without saving session details ourselves (ie. agent-shell--save-session). As of claude-code-acp 0.16.1, I was able to send this and get a list of all the sessions I ever started. lol. (acp-send-request
:client (map-elt (agent-shell--state) :client)
:request `((:method . "session/list")
(:params . ((cwd . ,(agent-shell--resolve-path (string-remove-suffix "/" (agent-shell-cwd)))))))
:buffer (current-buffer)
:on-success (lambda (response)
(message "response ->>> %s" response))
:on-failure (lambda (_error _raw-message)
(message "error ->>> %s" _error)))No request from my end. Just sharing things as I continue digging in. |
2 participants
Summary
Changes
.agent-shell/sessions/agent-shell-resume-session,agent-shell-delete-saved-session{}which json-read parses as nil)Details
The ACP protocol indicates session capabilities (like
resume) by the presence of a key with an empty object{}, not a boolean. When Emacs parses this withjson-read, the empty object becomesnil. The fix usesmap-contains-keyto check for key existence rather than value truthiness.New customization options:
agent-shell-session-persistence-enabled- Toggle session persistence (default: t)agent-shell-sessions-directory-function- Customize storage directoryTest plan
.agent-shell/sessions/M-x agent-shell-resume-sessionCloses #105
Depends on: xenodium/acp.el#11 for
acp-make-session-resume-request🤖 Generated with Claude Code